fix: use core instead of std - #741
Conversation
| /// | ||
| /// // Existing key (or_insert) | ||
| /// let v = map.entry("b").or_insert(2); | ||
| /// assert_eq!(std::mem::replace(v, 2), 20); |
There was a problem hiding this comment.
Mostly in favour of this change, but I would rather not update the doctests specifically since in general we prefer making those as simple as possible, and using only std allows better import grouping.
There was a problem hiding this comment.
Import grouping shouldn’t be an issue here. In the doctests, we only use the following std imports, and they are never used in combination with core:
/// use std::hash::RandomState;
/// use std::rc::Rc;
| use std::collections::BTreeSet; | ||
| use std::panic::catch_unwind; | ||
| use std::sync::Mutex; |
There was a problem hiding this comment.
Would brace-group these:
| use std::collections::BTreeSet; | |
| use std::panic::catch_unwind; | |
| use std::sync::Mutex; | |
| use std::{ | |
| collections::BTreeSet, | |
| panic::catch_unwind, | |
| sync::Mutex, | |
| }; |
There was a problem hiding this comment.
I had considered that, but rustfmt wanted to remove whitespace which I didn't like:
use std::{collections::BTreeSet, panic::catch_unwind, sync::Mutex};There was a problem hiding this comment.
I see; personally, I still would go with it, but it's up to you.
There was a problem hiding this comment.
In an ideal world, we'd use imports_layout = "Vertical" like libc but it's an unstable feature.
There was a problem hiding this comment.
@clarfonthey By the way, would you prefer a fixup commit or rebase?
There was a problem hiding this comment.
For the main repo we don't want nested merge commits so a rebase would be ideal. Don't really mind if the result is multiple commits, although figured you'd also squash them as well anyway.
There was a problem hiding this comment.
If you want to propose new rustfmt.toml changes, wouldn't actually mind that, since formatting in CI is always via nightly. But probably better as a separate PR which can be ignored on blame.
There was a problem hiding this comment.
Right, yeah. Since the repo doesn’t use squash merges, I think a rebase is needed despite any reviewer preference.
`std::collections::hash_map::RandomState` moved to `std::RandomState` in Rust 1.76.
89b0299 to
0119920
Compare
|
FWIW: there are clippy lints for |
|
At the moment, LGTM minus the clippy lints to help avoid this in the future. |
2 participants
No description provided.